AO3-7231 Upgrade gems and configs to Rails 8.0.x #5515

marcus8448 · 2025-12-19T05:50:29Z

Issue

https://otwarchive.atlassian.net/browse/AO3-7231

Purpose

Upgrade to Rails 8.0.

The upgrade was surprisingly (suspiciously?) straightforward, considering it's a major version bump. I'll add additional notes/comments in a review.

Testing Instructions

TBD, see Jira.

Credit

marcus8448 (he/him)

Looked at all uses of to_time: * used for duration (difference, so zone vs offset doesn't matter) * stored in model/DB (rails converts to UTC) * read from model and sent to akismet (rails converts to UTC) * or just forced to utc strict_freshness: I don't see any issues arising from it Regexp.timeout: in my mind, 1s should be sufficient

better safe than sorry?

marcus8448

Upgrade script wanted puma.rb, but I ignored it per this comment.

It also wanted to set silence_healthcheck_path in production.rb but it doesn't look like we're using the system anyways. (Noting this just in case we want something silenced, but I doubt it.)

marcus8448 · 2025-12-19T05:57:28Z

Gemfile


 # Library for helping run pt-online-schema-change commands:
-gem "departure", "~> 6.8"
+gem "departure", "~> 8.0"


Didn't see anything of note in the changelog. Note that 8.0.1 seems to have not been published to rubygems (if it does get published I would like to pull it in).

marcus8448 · 2025-12-19T05:59:43Z

Gemfile


 group :test do
-  gem "rspec-rails", "~> 6.0"
+  gem "rspec-rails", "~> 8.0"


Changelog, nothing stood out as well.

marcus8448 · 2025-12-19T06:09:55Z

app/models/tagset_models/tag_set.rb

Rails 8.0 now saves the parent model (tag set) before the child model (owned tag set), so we can't always touch it here (after_save). In that case the child model (owned tag set) is going to be saved after anyways, so it's fine to just skip touching.

This change looks to be a side effect of rails/rails#49847, but I think this ordering makes more sense so we were probably just depending on buggy behaviour.
Example, if that explanation doesn't make sense: https://gist.github.com/marcus8448/9459bcc0536f2baf0704cbea91fc9d0b

Looking at your reproduction, did you have a poke around whether any other models are affected by the parent after save/after create being moved around?

(Feel free to just say "no" and we will rely on the automatic tests)

marcus8448 · 2025-12-19T06:21:16Z

config/environments/development.rb

-  # Raise exceptions for disallowed deprecations.
-  config.active_support.disallowed_deprecation = :raise
-
-  # Tell Active Support which deprecation messages to disallow.
-  config.active_support.disallowed_deprecation_warnings = []
-


Removed from example config, we're not using it anyways.

config/environments/development.rb

marcus8448 · 2025-12-19T06:23:29Z

config/environments/development.rb

+  # Append comments with runtime information tags to SQL queries in logs.
+  config.active_record.query_log_tags_enabled = true
+


Example (the /**/ comment):

Tag Load (0.2ms) SELECT `tags`.* FROM `tags` WHERE `tags`.`name` = 'Uncategorized Fandoms' LIMIT 1 /*action='index',application='Otwarchive',controller='home'*/

https://guides.rubyonrails.org/configuring.html#config-active-record-query-log-tags-enabled

When this is set to true database prepared statements will be automatically disabled.

I'd prefer to keep prepared statements in dev by default. But we can still have this line in the config, just commented out

config/environments/development.rb

config/initializers/filter_parameter_logging.rb

marcus8448 · 2025-12-19T06:27:09Z

public/400.html

Not sure if this file is needed since the app should be able to respond itself.

These get copied to the Nginx proxy/frontend servers, so if memory serves, they can respond with this rather than going to the backend for it

The setting will be undeprecated and enabled by default in Rails 8.2 https://redirect.github.com/rails/rails/commit/a477a3273c3c71305cc8ae1835638dc75184ad9d

marcus8448 · 2026-01-10T06:31:26Z

Gemfile

-gem "rails", "~> 7.2"
-gem "rails-i18n"
+gem "rails", "~> 8.0.0"
+gem "rails-i18n", "~> 8.0", git: "https://github.com/svenfuchs/rails-i18n", ref: "54c1c7c2fdcc311427ec6f1dadd298a60db1ddef"


Crashes without an unreleased bugfix. Technically this version targets rails 8.1, but it looks like there are only new keys for 8.1 (none changed/removed) so it shouldn't cause any problems.

Please do the rails-i18n upgrade steps in https://github.com/otwcode/otwarchive/blob/master/config/locales/README.md

marcus8448 · 2026-01-10T06:38:49Z

config/initializers/active_job_enqueue_after_commit.rb

+# always enable enqueue_after_transaction_commit
+# TODO: remove for rails 8.2! https://github.com/rails/rails/commit/a477a3273c3c71305cc8ae1835638dc75184ad9d
+Rails.application.config.after_initialize do
+  ActiveSupport.on_load(:active_job) do
+    ActiveJob::Base.enqueue_after_transaction_commit = true
+  end
+end


This is a manual re-implementation of what the deprecated config.active_job.enqueue_after_transaction_commit = :always did.

The global option is going to be undeprecated and enabled by default in Rails 8.2, so I don't see any reason to not continue with the current behaviour.

Very odd... when you write the testing notes, could you see if we can definitely hit this to make sure it works like we expect?

I don't think we can hit this in manual testing? We didn't for 7.2 where I enabled this value: #5303 (comment)

Note that I haven't tried this, but could we set enqueue after commit directly on ApplicationJob (without all the initializer stuff) since all our jobs descend from that?

marcus8448 · 2026-01-10T06:49:41Z

config/application.rb

    # in config/environments, which are processed later.

-    config.load_defaults 7.2
+    config.load_defaults 8.0


New framework defaults

### # Specifies whether `to_time` methods preserve the UTC offset of their receivers or preserves the timezone. # If set to `:zone`, `to_time` methods will use the timezone of their receivers. # If set to `:offset`, `to_time` methods will use the UTC offset. # If `false`, `to_time` methods will convert to the local system UTC offset instead. #++ Rails.application.config.active_support.to_time_preserves_timezone = :zone

I looked at all the uses of to_time and nothing stood out as being timezone (vs offset) dependent. Anything coming into or out of the database is normalized to UTC anyways and duration calculations shouldn't care about the zone.

### # When both `If-Modified-Since` and `If-None-Match` are provided by the client # only consider `If-None-Match` as specified by RFC 7232 Section 6. # If set to `false` both conditions need to be satisfied. #++ Rails.application.config.action_dispatch.strict_freshness = true

I don't see anything wrong with this.

### # Set `Regexp.timeout` to `1`s by default to improve security over Regexp Denial-of-Service attacks. #++ Regexp.timeout = 1

I think 1s should be plenty of time. Do we have any important complex regexps?

to_time: I'm slightly concerned about the use in time_ago_in_words, since it uses Time.now which is not zone aware. Can we switch it to Time.current just for safety? Servers and Rails are in UTC (they weren't always...) so nothing should break, but just in case.

Do we have any important complex regexps?

not sure if complex, but word counting is very important, likely to have very large input and is a regex (ref WordCounter). Maybe test it locally with a very very long chapter and see whether it's an issue?

brianjaustin

Some minor things, and I'd like to see testing steps in the ticket before approving. Looks good, generally, though!

brianjaustin · 2026-01-14T18:39:24Z

app/controllers/errors_controller.rb

@@ -1,5 +1,5 @@
 class ErrorsController < ApplicationController
-  %w[403 404 422 500].each do |error_code|
+  %w[400 403 404 422 500].each do |error_code|


Is this related to the new version, or just completeness?

More for completeness - the new version just wanted /public/400.html (relevant commit), but when making a copy with the archive styling I took a look at what we did for error 500 and saw this controller. If the handling is redundant considering the static pages then I can just delete it.

In my opinion we should decide on one place to handle it, probably the public file, not here.

Since there are multiple error codes here that have been there already, let's do this in a follow-up ticket (something like "standardise HTTP error handling" maybe)

I mean for this one specific new error 400, just don't add it to the ErrorsController. The other errors are fine to stay here

public/400.html

brianjaustin · 2026-01-14T18:41:44Z

public/400.html

These get copied to the Nginx proxy/frontend servers, so if memory serves, they can respond with this rather than going to the backend for it

brianjaustin · 2026-01-14T18:42:37Z

config/initializers/active_job_enqueue_after_commit.rb

+# always enable enqueue_after_transaction_commit
+# TODO: remove for rails 8.2! https://github.com/rails/rails/commit/a477a3273c3c71305cc8ae1835638dc75184ad9d
+Rails.application.config.after_initialize do
+  ActiveSupport.on_load(:active_job) do
+    ActiveJob::Base.enqueue_after_transaction_commit = true
+  end
+end


Very odd... when you write the testing notes, could you see if we can definitely hit this to make sure it works like we expect?

brianjaustin

Thanks!

Bilka2

Marking as requesting changes for the to_time thing, regex complexity manual test and rails-i18n upgrade, rest of my comments are optional to address

marcus8448 added 7 commits December 17, 2025 21:00

AO3-7231 Bump Rails version, run update task

fecbdd4

AO3-7231 Temporarily use alternative rails-i18n source

8fd90f0

AO3-7231 Don't touch owned tag set if it hasn't been persisted yet

b119a34

AO3-7231 Set 8.0 defaults

0f97a02

AO3-7231 Try deleting enqueue_after_transaction_commit?

691cfe8

AO3-7231 Set enqueue_after_transaction_commit on jobs

579fc0e

better safe than sorry?

github-actions bot added Gem Updates Awaiting Review labels Dec 19, 2025

marcus8448 commented Dec 19, 2025

View reviewed changes

marcus8448 added 4 commits December 18, 2025 22:54

AO3-7231 Rubocop

8bf5c17

Merge branch 'master' into AO3-7231

7e33f1e

AO3-7231 Pull from main rails-i18n repository

3c079f5

AO3-7231 Always enable enqueue_after_transaction_commit

c3f8b1c

The setting will be undeprecated and enabled by default in Rails 8.2 https://redirect.github.com/rails/rails/commit/a477a3273c3c71305cc8ae1835638dc75184ad9d

marcus8448 commented Jan 10, 2026

View reviewed changes

brianjaustin reviewed Jan 14, 2026

View reviewed changes

brianjaustin added Reviewed: Action Needed and removed Awaiting Review labels Jan 14, 2026

brianjaustin approved these changes Jan 15, 2026

View reviewed changes

Bilka2 requested changes Jan 16, 2026

View reviewed changes

		# Append comments with runtime information tags to SQL queries in logs.
		config.active_record.query_log_tags_enabled = true

AO3-7231 Upgrade gems and configs to Rails 8.0.x #5515

Are you sure you want to change the base?

AO3-7231 Upgrade gems and configs to Rails 8.0.x #5515

Uh oh!

Conversation

marcus8448 commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue

Purpose

Testing Instructions

Credit

Uh oh!

marcus8448 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Bilka2 Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Bilka2 Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcus8448 Jan 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Bilka2 Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Bilka2 Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brianjaustin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Bilka2 Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcus8448 commented Dec 19, 2025 •

edited

Loading

marcus8448 left a comment •

edited

Loading

Bilka2 Jan 16, 2026 •

edited

Loading

Bilka2 Jan 16, 2026 •

edited

Loading

marcus8448 Jan 10, 2026 •

edited

Loading

Bilka2 Jan 16, 2026 •

edited

Loading

Bilka2 Jan 16, 2026 •

edited

Loading

Bilka2 Jan 16, 2026 •

edited

Loading

Bilka2 left a comment •

edited

Loading